Skip to content

build: fix up report execution container for test running#3075

Merged
mcmcgrath13 merged 4 commits intomainfrom
mcm/build-py-db-fix
Mar 12, 2026
Merged

build: fix up report execution container for test running#3075
mcmcgrath13 merged 4 commits intomainfrom
mcm/build-py-db-fix

Conversation

@mcmcgrath13
Copy link
Contributor

Description

In completing #3074 , I found some of the container set up wasn't actually working fully. This PR fixes that:

  • Add the mssql driver dependencies to the docker container
  • Use host networking for the integration test container run so both the tests directly and the repot-execution container can talk to the DB using the same localhost connection string
  • Add the DATABASE_CONN_STRING env var to the base compose and sample env
  • Set up the docker compose in integration tests to use the sample env files
  • Have the integration docker compose use the --build flag when setting up containers
  • Add an integration test to prove it works

@mcmcgrath13 mcmcgrath13 requested a review from a team as a code owner March 10, 2026 15:20
@mcmcgrath13 mcmcgrath13 requested review from brick-green and emyl3 and removed request for a team March 10, 2026 15:20

# service settings
NBS_DATASOURCE_SERVER=nbs-mssql
DATABASE_CONN_STRING="Server=${NBS_DATASOURCE_SERVER};UID=${DATABASE_USER};PWD=${DATABASE_PASSWORD};Database=NBS_ODSE;TrustServerCertificate=yes;"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OOO yes and making a new .env with this also fixes the internal server error I was getting when trying to make a request to the report execution service when starting it up in Docker! 🎉

@sonarqubecloud
Copy link

Copy link
Contributor

@emyl3 emyl3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing!!!

@mcmcgrath13 mcmcgrath13 merged commit 5a1de13 into main Mar 12, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants